Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support :focus for rspec permissions blocks #820

Merged

Conversation

StevenElberger
Copy link
Contributor

I thought this issue would be nice to have so I took a stab at it:
#816

Please let me know if there's anything missing!

@Burgestrand
Copy link
Member

Spontaneously this looks good! I'm currently enjoying Swedish vacation so I'll take a proper look in august 😊

@StevenElberger StevenElberger force-pushed the feature/support-rspec-focus-filter branch from b5b7825 to 92c900d Compare July 25, 2024 04:53
Copy link
Member

@Burgestrand Burgestrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I think the implementation is good! Readme needs some tiny adjustments, and the test needs a few changes too. Please see my comments 😊

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
spec/dsl_spec.rb Outdated Show resolved Hide resolved
spec/dsl_spec.rb Outdated Show resolved Hide resolved
spec/dsl_spec.rb Outdated Show resolved Hide resolved
@StevenElberger StevenElberger force-pushed the feature/support-rspec-focus-filter branch from 92c900d to 36dd293 Compare August 8, 2024 05:47
@StevenElberger
Copy link
Contributor Author

@Burgestrand thank you so much for your very helpful and insightful feedback! 🙇 I hope the latest changes satisfy your comments but if not please let me know!

spec/dsl_spec.rb Outdated Show resolved Hide resolved
@StevenElberger StevenElberger force-pushed the feature/support-rspec-focus-filter branch 2 times, most recently from 766e8bc to 3c529a8 Compare August 10, 2024 04:22
Burgestrand
Burgestrand previously approved these changes Aug 12, 2024
Copy link
Member

@Burgestrand Burgestrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks great!

@Burgestrand
Copy link
Member

Burgestrand commented Aug 12, 2024

@StevenElberger rubocop caught a missing frozen string literal comment! Once you add that I believe this is ready to merge 😊

@StevenElberger
Copy link
Contributor Author

@StevenElberger rubocop caught a missing frozen string literal comment! Once you add that I believe this is ready to merge 😊

@Burgestrand got it! should be good to go now, thank you! 😃

@Burgestrand Burgestrand merged commit 8992430 into varvet:main Aug 13, 2024
14 checks passed
@Burgestrand
Copy link
Member

Burgestrand commented Aug 13, 2024

@StevenElberger merged, thank you! I'll see if I can get a release going with this change this month 😊

Burgestrand added a commit that referenced this pull request Aug 26, 2024
## Improve `NotAuthorizedError` message to include policy class (#812)

Default error message changed from:
> not allowed to destroy? this Comment

To include the policy class:
> not allowed to Project::Admin::CommentPolicy#destroy? this Comment

## Improve `NotAuthorizedError` when record is a class

Before:
> not allowed to index? this Class

After:
> not allowed to PostPolicy#index? Post

## Allow customizing rspec matcher description (#806)

Before:
> PostPolicy
>  update? and show?
>    is expected to permit #<User:0x0000000104aefd80> and #<Post:0x0000000104aef8d0 @user=#<User:0x0000000104aefd80>>

In `spec_helper.rb`:
```ruby
Pundit::RSpec::Matchers.description = ->(user, record) do
  "permit user with role #{user.role} to access record with ID #{record.id}"
end
```

After:
> PostPolicy
>  update? and show?
>    is expected to permit user with role admin to access record with ID 130

## Add support for filter_run_when_matching :focus with permissions helper (#820)

If your RSpec config has filter_run_when_matching :focus, you may tag the permissions helper like so:

```ruby
permissions :show?, :focus do
```
@Burgestrand Burgestrand mentioned this pull request Aug 26, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants